refactor: extract the source study citation into its own component (#3218) by frano-m · Pull Request #3224 · DataBiosphere/data-portal

frano-m · 2026-09-10T07:15:39Z

Summary

Closes #3218.

FileNameCell guarded its citation line on the result of the split rather than on the citation itself. #3218 raised three consequences and offered the shape that fixes all three: a subcomponent taking a required publicationString: string.

// before
const citation = publicationString ? splitTrailingWord(publicationString) : null;
{citation && ( … {doi ? <Link label={…citation.head…} /> : publicationString} … )}

// after
{publicationString && ( … <Citation doi={doi} publicationString={publicationString} /> … )}

The three points, and how each is now closed

#	Before	After
1. Indirect guard	`{citation && …}` guarded a block rendering bare `publicationString`	guard is `{publicationString && …}` — the thing itself
2. Narrowing lost	`publicationString` stayed `string \| null` inside the guard	narrows to `string`, and is passed as a required prop
3. Split ran unused	built and discarded on rows with a citation but no DOI	`if (!doi) return …` sits above the split

Point 2 demonstrated rather than asserted: changing the call site to publicationString.trimEnd() now compiles with no null assertion, where before it produced TS18047: 'publicationString' is possibly 'null'.

The split is still called once per render, so #3213's fix is preserved rather than undone.

Structure

The citation-only pieces moved with it, so the concern is self-contained and fileNameCell.tsx is back to a Stack plus two lines:

FileNameCell/
  fileNameCell.tsx              Stack + file name + <Citation/>
  types.ts
  components/Citation/
    citation.tsx                takes publicationString: string
    citation.styles.ts          StyledNoWrap, StyledOpenInNewIcon   (moved)
    constants.ts                DOI_BASE_URL, EXTERNAL_LINK_TITLE   (moved)
    types.ts
    utils.ts                    splitTrailingWord                    (moved)

All three moves are 100%-similarity renames, so history follows them. Folder naming matches the sibling AnalysisPortalCell convention.

Verification

Rendered output is unchanged:

7 DOI anchors, all 7 carrying the icon inside the anchor
7 nowrap spans, each holding the citation's final word plus its <svg>
7 (opens in a new tab) titles — the titleAccess a11y cue survives
href="" count 0
citation still var(--palette-ink-light); icon still 16px
7 file names on the primary line, 6 HCA Explorer links
npm run lint (0 errors), npm run check-format, npx tsc --noEmit, npm run build-prod:data-portal (64/64) all pass

One equivalence worth spelling out, raised in review: for publicationString === "" the old guard produced null and rendered nothing; the new guard is falsy and also renders nothing. Equivalent for every value of string | null.

Not done

splitTrailingWord is a pure function with edge cases that deserve unit tests — single word, trailing whitespace, empty string — and has none. It moved unchanged, so this is pre-existing, and there is still no test tooling in the repo (#3190). Worth seeding alongside buildVersionedFileName and splitFileName when that lands.

No conflict with the open PRs

Checked before starting: this touches only FileNameCell/. #3223 touches @types/network.ts, constants/networks.ts and two content/breast/ files; #3216 touches viewModelBuilders.tsx. No overlap either way, so this sits on main rather than stacking.

🤖 Generated with Claude Code

Copilot

🔵 Needs a closer look

splitTrailingWord currently slices the untrimmed input after computing an index from trimEnd(), which can produce a tail that includes trailing whitespace and contradicts the function’s documented behavior.

Pull request overview

This PR refactors the FileNameCell citation rendering by extracting the DOI/link + nowrap/icon logic into a dedicated Citation subcomponent, aligning the guard/narrowing behavior with publicationString and keeping the split work scoped to the DOI branch.

Changes:

Updated FileNameCell to guard on publicationString and delegate citation rendering to a new <Citation /> component.
Introduced Citation component with supporting constants, styles, types, and splitTrailingWord utility colocated under FileNameCell/components/Citation/.
Moved the DOI/link-related styles and constants out of fileNameCell.tsx into the citation-specific folder for better separation of concerns.

File summaries

File	Description
components/HCABioNetworks/Network/Atlas/components/SourceDatasets/components/MainColumn/components/table/components/FileNameCell/fileNameCell.tsx	Simplifies cell rendering and delegates citation logic to `<Citation />`.
components/HCABioNetworks/Network/Atlas/components/SourceDatasets/components/MainColumn/components/table/components/FileNameCell/components/Citation/citation.tsx	New component encapsulating DOI-link citation rendering and icon/nowrap behavior.
components/HCABioNetworks/Network/Atlas/components/SourceDatasets/components/MainColumn/components/table/components/FileNameCell/components/Citation/citation.styles.ts	Moves and scopes citation-specific styled components (nowrap span + icon styling).
components/HCABioNetworks/Network/Atlas/components/SourceDatasets/components/MainColumn/components/table/components/FileNameCell/components/Citation/constants.ts	Centralizes DOI base URL and external-link accessibility title.
components/HCABioNetworks/Network/Atlas/components/SourceDatasets/components/MainColumn/components/table/components/FileNameCell/components/Citation/types.ts	Defines props contract for the extracted `Citation` component.
components/HCABioNetworks/Network/Atlas/components/SourceDatasets/components/MainColumn/components/table/components/FileNameCell/components/Citation/utils.ts	Houses `splitTrailingWord` used to keep the last word + icon unbroken.

Review details

Files reviewed: 3/6 changed files
Comments generated: 0
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

frano-m · 2026-09-10T08:03:39Z

Good catch — real bug, fixed in bd8a436.

The index came from trimEnd() but the slices came from the original string, so trailing whitespace landed on the tail. Verified by running the function directly:

in="Kumar (2023) Nature  "   head="Kumar (2023) "   tail="Nature  "   <- trailing spaces on the tail
in="Nature  "                head=""                tail="Nature  "

That matters because the tail sits immediately before the icon inside the nowrap span, so it renders as a gap wider than the icon's deliberate margin-left: 4px. And as you say it contradicted the JSDoc, which promises "the final word".

Fixed by slicing the trimmed value, so head + tail is the trimmed input:

const trimmed = text.trimEnd();
const index = trimmed.lastIndexOf(" ");
if (index === -1) return { head: "", tail: trimmed };
return { head: trimmed.slice(0, index + 1), tail: trimmed.slice(index + 1) };

Compared old against new across the cases — only trailing-whitespace inputs change, every real citation is byte-identical:

input	old tail	new tail
`"Gray (2022) Developmental Cell"`	`"Cell"`	`"Cell"`
`"Twigger (2022) Nature Communications"`	`"Communications"`	`"Communications"`
`"Kumar (2023) Nature "`	`"Nature "`	`"Nature"`
`"Nature "`	`"Nature "`	`"Nature"`
`""`	`""`	`""`
`" "`	`" "`	`""`

Rendered output confirms it: the six distinct tails are Cell, Communications, Genetics, Journal, Nature, Systems, none with trailing whitespace, and all 7 anchors still carry the icon and the (opens in a new tab) cue.

Worth noting the whitespace path is unexercised by live data — no Breast v1.0 citation has trailing whitespace — so this was latent rather than visible. It is also exactly the kind of edge case the earlier review flagged as deserving unit tests for this function; there is still no test tooling in the repo (#3190), so I verified by executing the function directly rather than adding a test that cannot run.

🤖 Generated with Claude Code

…3218) Filenamecell guarded the citation on the split result rather than on publicationstring, so the guard read indirectly, typescript stopped narrowing the string the no-doi branch renders, and the split ran on rows that never used it. A citation component taking a required publicationstring puts all three right: the caller owns whether there is a citation, the string arrives narrowed, and the split sits in the doi branch that consumes it. The citation-only pieces move with it, so filenamecell is now a stack and two lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The index came from trimend() but the slices came from the original, so any trailing whitespace landed on the tail - which sits immediately before the external-link icon, rendering as a gap wider than the icon's deliberate 4px margin. Also contradicted the jsdoc, which promises the final word. Only trailing-whitespace inputs change; every real citation is unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

NoopDog

Approving — the extraction is clean, callers all resolve, tsc --noEmit and npm run lint pass, and the rendered output is unchanged for the current data.

Two low-priority notes, neither blocking:

1. Citation/utils.ts — lastIndexOf(" ") only matches U+0020.
A citation whose word separators are non-breaking spaces, tabs, or newlines (plausible for a publicationString pasted from a typeset journal page) returns -1, so head is "" and the whole citation lands inside StyledNoWrap (white-space: nowrap). In the pinned Source Datasets column that overflows/clips the cell instead of wrapping to a second line — the opposite of what the helper exists to prevent. Pre-existing behaviour, but since the line was rewritten here it's cheap to close: match any whitespace, e.g. /\s(?=\S*$)/.

2. The PR description undersells one hunk.
The "Not done" section says splitTrailingWord "moved unchanged", but 3642595 slices tail from text.trimEnd(), so head + tail === text.trimEnd() rather than text. The change is correct and a genuine improvement — a trailing space inside a nowrap span renders as a visible gap before the icon — it just isn't part of the 100%-similarity renames, and a reviewer skimming that claim would skip the one hunk with behaviour in it.

🤖 Generated with Claude Code

https://claude.ai/code/session_018z2yTzwXdsj9qfjxzTHTk7

frano-m assigned NoopDog Sep 10, 2026

frano-m force-pushed the fran/3218-citation-subcomponent branch from 7445807 to e3ed9ad Compare September 10, 2026 07:20

frano-m requested a lite review from Copilot September 10, 2026 07:58

Copilot started reviewing on behalf of frano-m September 10, 2026 07:59 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

This was referenced Sep 10, 2026

refactor: FileNameCell guards the citation on the split result rather than on publicationString #3218

Open

refactor: guard FileNameCell's citation on publicationString so the string narrows #3219

Closed

frano-m and others added 2 commits September 11, 2026 09:07

frano-m force-pushed the fran/3218-citation-subcomponent branch from bd8a436 to 3642595 Compare September 10, 2026 23:07

NoopDog approved these changes Sep 11, 2026

View reviewed changes

Navigation Menu

Uh oh!

refactor: extract the source study citation into its own component (#3218) - #3224

refactor: extract the source study citation into its own component (#3218)#3224
frano-m wants to merge 2 commits into
mainfrom
fran/3218-citation-subcomponent

frano-m commented Sep 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

frano-m commented Sep 10, 2026

Uh oh!

NoopDog left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

frano-m commented Sep 10, 2026

Summary

The three points, and how each is now closed

Structure

Verification

Not done

No conflict with the open PRs

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

frano-m commented Sep 10, 2026

Uh oh!

NoopDog left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants